Repository navigation
Conversation
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
In compare mode, a set of calendar blocks that overlap each other now always
divides its day column evenly, with no empty column left between or beside
them. Two people busy at the same time each get half the column, however many
other (non-concurrent) blocks that day happens to contain. The regular
(non-compare) Scheduler calendar is untouched — its packing code runs exactly
as before.
Compare mode sizes a day's columns by walking the connected component of
overlapping blocks and stamping every block in that component with the same
column count. A connected component is not the same thing as "how many people
are busy at once": blocks chain together (A overlaps B, B overlaps C, A and C
never overlap), and the whole chain is then sized to the length of the chain
rather than to the largest number of blocks that are ever concurrent.
Before. Pin your own 9:30–10:45 section and a friend's 10:00–11:15 block:
they correctly split the Monday column in half. Now toggle on a second friend
whose block runs 11:00–12:15 — it overlaps the first friend but not you. All
three blocks immediately shrink to a third of the column, and a visibly empty
third opens up next to each of them. Nothing about your 9:30 conflict changed,
but it now renders narrower and further from the block it actually conflicts
with. The more schedules you overlay, the longer the chains get and the worse
it looks: the grid reads as far more crowded than it is, and it becomes harder
to see which blocks genuinely conflict.
After. Each of those blocks keeps half the column, because only two people
are ever busy at the same instant. Toggling a third, non-concurrent schedule on
or off no longer resizes anything it does not actually overlap.
src/components/Calendar/index.tsxnow has two packing paths, selected by theexisting
compareprop:packMeetingsLegacyis the existing algorithm, moved verbatim out of thecomponent body into a module-level function (including its
updateJoinedRowSizeshelper). No behavior change.packMeetingsComparecollects one block per(id, day, period)and handseach day's blocks to
packDayBlocks.packDayBlockssorts a day's blocks by start time (ties broken by endtime, then by block key, so the result never depends on the order schedules
happened to be merged in) and assigns each block the leftmost column that is
free at the moment it starts. Blocks are grouped into maximal runs connected
by overlap; when a group closes, every block in it is given the same
rowSize— the number of columns that group actually needed.For time intervals, first-fit over start-sorted intervals uses exactly the
minimum number of columns, which is the maximum number of blocks concurrent at
any instant (this is optimal coloring of an interval graph). So a group of
mutually overlapping blocks always tiles its day column with no gaps, and the
answer is independent of insertion order.
packMeetingsComparealso de-duplicates blocks: a single schedule version thatis reachable through two different people is overlaid once per person, and the
two identical blocks are drawn on top of each other. It now claims one column
instead of two, so the duplicate no longer halves everyone's width.
Compare-only by construction. The non-compare path is guaranteed unchanged,
not merely believed to be:
apart from a
blockKey(block)helper that replaces the inline'crn' in x ? x.crn : x.idexpression, which is the same expression.if (compare), andcomparealready gatedcompare-mode behavior throughout this component.
mainproduces today (thirds of a column for a three-block chain, a fullcolumn for a lone block). They fail if the new packing ever leaks into the
Scheduler tab.
This fix is deliberately scoped to compare mode only, to limit risk to the one
place this was reported and keep the regular Scheduler's packing behavior
completely untouched. The same connected-component-vs-concurrent-overlap issue
plausibly affects the regular Scheduler too (see #130) — if maintainers want
it, the
packDayBlocksapproach here could likely be extended to the legacypath as a follow-up, but that's out of scope for this PR.
Notes for reviewers: no new dependencies, no changes to props, context,
styles, or any other component;
packDayBlocksmutates therowIndex/rowSizeof the block objects it's given, matching how the surrounding codealready builds
meetingSizeInfo.Resolves #450
Checklist
src/components/Calendar/index.test.tsx, new file — covers both compare-mode packing and non-compare lock-down)tsc --noEmit,eslint,prettier --checkon touched files)How to Test
CI=true yarn test --watchAll=false.(9:30–10:45), a friend's block (10:00–11:15), and a second friend's block
(11:00–12:15, which overlaps only the first friend, not you).
overlaps — here, all three should stay at half-column width with no dead
column, since no more than two are ever concurrent. On
maintoday, allthree incorrectly shrink to a third of the column with a visible gap.
Detail on what's covered in
src/components/Calendar/index.test.tsx: itrenders
Calendarinside stubScheduleContext/FriendContextproviderswith a one-section stub
Oscar, then reads theleft/widthinline stylesoff the rendered
.meetingelements — i.e. it asserts on what a user actuallysees, not on internals.
Compare mode:
day column), sitting at 0% and 10% with no gap.
concurrent → all four blocks stay 10% wide at the expected offsets. This is
the reported bug; on
mainthese blocks come out 6.67% wide.takes a single column, so widths stay at 10%.
Regular scheduler (lock-down):
20/3, 40/3 — the current
mainbehavior.Verified by reverting only
Calendar/index.tsxtomainand re-running thefile: tests 2 and 3 fail there (blocks come out 6.67% wide) while tests 4 and 5
pass, so the regression tests really do pin the reported behavior and the
lock-down tests really do describe today's Scheduler layout. Test 1 passes
either way — with only two blocks the old algorithm already got the right
answer, which is exactly why the bug went unnoticed until several schedules
were compared at once.
CI=true yarn test --watchAll=falseonmainbefore the change:and with the change:
— the whole existing suite still passes, plus the 5 new tests. Also clean:
tsc --noEmit,eslint, andprettier --checkon both touched files.